-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: allow arbitrary space indentation #7818
feat: allow arbitrary space indentation #7818
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's solve the discussion before concluding on PR
@@ -25,8 +25,8 @@ final class WhitespacesFixerConfig | |||
|
|||
public function __construct(string $indent = ' ', string $lineEnding = "\n") | |||
{ | |||
if (!\in_array($indent, [' ', ' ', "\t"], true)) { | |||
throw new \InvalidArgumentException('Invalid "indent" param, expected tab or two or four spaces.'); | |||
if (!Preg::match('/^(?: +|\t)$/', $indent)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IF we would go forward direction to approve the PR, we need a new tests to prove it works:
- each fixer that uses WhitespacesFixerConfig should receive a new test with new whitespace variant(s)
- dedicated integration tests for new whitespace variant(s) (at least mimic
tests/Fixtures/Integration/set/@PSR12_whitespaces.test*.php
)
(i suggest to not invest into it till aligned into direction)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll convert this to a draft while I familiarize myself more with the codebase.
Since this pull request has not had any activity within the last 90 days, I have marked it as stale. The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way. You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point. I will close it if no further activity occurs within the next 14 days. Please keep your branch up-to-date by rebasing it when main branch is ahead of it, thanks in advance! |
The fact this was automatically closed doesn't mean that the idea got rejected - it simply didn't get any priority for way too long to keep it open. If you're still interested in this, please let as know, we can consider re-opening it. When it comes to pull requests it may be better to create new one, on top of main branch. |
I created this PR to address the issue in #7810.
Removes the limitation of having only 2 or 4 space indentation and allows for any number of spaces or one tab in the indentation settings.
This could be modified to also allow for any number of tabs but I don't see a reason for it since tabs are already arbitrary sized.